fix(tooluse): always call model to prevent tooluse injection#2136
Open
poshinchen wants to merge 1 commit intostrands-agents:mainfrom
Open
fix(tooluse): always call model to prevent tooluse injection#2136poshinchen wants to merge 1 commit intostrands-agents:mainfrom
poshinchen wants to merge 1 commit intostrands-agents:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Remove
_has_tool_use_in_latest_messageto prevent tool execution bypass via injected toolUse blocksPR #1068 introduced
[_has_tool_use_in_latest_message()](https://github.com/strands-agents/sdk-python/blob/117da677fa398fffbce834d7df5099047b31eb82/src/strands/event_loop/event_loop.py#L58)in event_loop.py, which checks whether the latest message in the conversation contains toolUse content blocks and, if so, skips model invocation entirely and proceeds directly to tool execution.This check does not distinguish between a message produced by the model and one injected by an external caller.
Combined with _convert_prompt_to_messages(), which appends the messages directly to the conversation.
An attacker can inject a message containing toolUse blocks that execute immediately without the model ever being consulted. The system prompt, guardrails, and all model-level security controls are completely bypassed.Fix
This PR adopts the deferred-append pattern already used in the TypeScript SDK:
Remove
_has_tool_use_in_latest_message()so the model is always called; no user-supplied message can short-circuit into tool execution.Defer assistant message append:
Dangling toolUse handling: if
agent.messagesends with a dangling toolUse (e.g. from session restore), _convert_prompt_to_messages() appends a dummy error toolResult and the model is called normally.The only remaining path that skips model invocation is the interrupt state, which has proper provenance. The tool_use_message is set by the event loop itself, not from user input.
Additional changes
The
max_tokenshandling was also moved from _handle_model_execution to event_loop_cycle for consistency with the deferred-append pattern. The recovered message is now appended and MessageAddedEvent is fired before MaxTokensReachedException is raised.Breaking change
MessageAddedEvent for assistant messages now fires after tool execution (alongside the tool result) instead of before. Hooks that depend on seeing the assistant message in agent.messages during tool execution will need to adapt.
Related Issues
N/A
Documentation PR
N/A
Type of Change
Bug fix
Breaking change
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.